Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invisible tiles #7

Merged
merged 24 commits into from
Jun 28, 2023
Merged

Invisible tiles #7

merged 24 commits into from
Jun 28, 2023

Conversation

emilk
Copy link
Member

@emilk emilk commented Jun 17, 2023

This PR implements is_visible/set_visible for all tiles. Tiles are visible by default.

It also refactors the Grid container to avoid inadvertently re-arranging the order of its children.

@emilk emilk added the enhancement New feature or request label Jun 17, 2023
@emilk emilk marked this pull request as draft June 25, 2023 00:51
@emilk emilk marked this pull request as ready for review June 25, 2023 22:44
@Wumpf Wumpf self-requested a review June 26, 2023 08:12
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking great!

Comment on lines +43 to +55
if let Some(active) = self.active {
if !tiles.is_visible(active) {
self.active = None;
}
}

if !self.children.iter().any(|&child| self.is_active(child)) {
// Make sure something is active:
self.active = self.children.first().copied();
self.active = self
.children
.iter()
.copied()
.find(|&child_id| tiles.is_visible(child_id));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of making the first visible active, it would be nice if, whenever possible, the first visible item left of the currently active would become active.
Concrete: When I make the currently active tab invisible, I the left-most visible becomes active. This feels a bit off, I'd expect a neighboring element to to get the active state.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree that would be preferable, but it requires a bit more work… PRs welcome? :)

src/container/linear.rs Outdated Show resolved Hide resolved
Constructing two trees the same way will now yield equal trees
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest changes still looking good to me

@emilk emilk merged commit 0127ad5 into main Jun 28, 2023
8 checks passed
@emilk emilk deleted the emilk/invisible-tiles branch June 28, 2023 06:05
emilk added a commit to rerun-io/rerun that referenced this pull request Jun 28, 2023
Closes #2140

Sibling PR (that also needs a review!):
* rerun-io/egui_tiles#7

### What
Show the layout container views in the blueprint tree view:

<img width="196" alt="Screenshot 2023-06-24 at 04 29 43"
src="https://github.com/rerun-io/rerun/assets/1148717/27dddd22-dbda-496c-9637-ef218890a9e1">

### Future work
* Being able to select containers
* Drag-and-drop between them
* Better default layout

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2465

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/60b32df/docs
Examples preview: https://rerun.io/preview/60b32df/examples
<!-- pr-link-docs:end -->
emilk added a commit to rerun-io/rerun that referenced this pull request Jun 29, 2023
Closes #2140

Sibling PR (that also needs a review!):
* rerun-io/egui_tiles#7

### What
Show the layout container views in the blueprint tree view:

<img width="196" alt="Screenshot 2023-06-24 at 04 29 43"
src="https://github.com/rerun-io/rerun/assets/1148717/27dddd22-dbda-496c-9637-ef218890a9e1">

### Future work
* Being able to select containers
* Drag-and-drop between them
* Better default layout

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2465

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/60b32df/docs
Examples preview: https://rerun.io/preview/60b32df/examples
<!-- pr-link-docs:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants